-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved shared context. Refactor & simplify some specs. #7970
Conversation
e5cf9b6
to
866bfe1
Compare
Or maybe I could put these with the |
I think this looks like a great improvement to the spec seen in this PR. 👍 |
I have changed my PR. I believe this shared context is very flexible and would be a good replacement for |
let
to cop_helper. Refactor & simplify some specs.
I like the idea overall, but I'm not sure all the cops need so many things - you've picked to apply the suggest approach to two specs that are exactly typical cop specs. Perhaps we can get away with a smaller and more focused context for the actual cops. |
a2952c0
to
66bf72f
Compare
Now that I'm more familiar with the It streamlines the
I simplified a few other specs. I also removed ~150 identical and redundant I'll open an different issue for things I dislike about our current |
Looks good! Thanks! |
After rubocop/rubocop#7970, all specs with the `:config` metadata by default use the default configuration. So now, there is often no reason to add a cop's default configuration in specs.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
rubocop/rubocop#7970 in RuboCop 0.84 mixes in the default config, which changes the config options from "defult" (missing) `false` to actual `true` from config/defaults.yml
rubocop/rubocop#7970 in RuboCop 0.84 mixes in the default config, which changes the config options from "defult" (missing) `false` to actual `true` from config/defaults.yml
After rubocop/rubocop#7970, all specs with the `:config` metadata by default use the default configuration. So now, there is often no reason to add a cop's default configuration in specs.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
In rubocop/rubocop#7970 the shared context `config` was updated to include the following: let(:cop) do cop_class.new(config, cop_options) .tap { |cop| cop.processed_source = processed_source } end This is functionally equivalent to what we have all over our specs: subject(:cop) { described_class.new(config) } The downside is that our specs become a less explicit. Actually they now have a mystery guest, which may be considered a smell.
I feel there are ways to clean some specs up with default rspec
let
groups.How does this look for a start?
Stumbling upon this when working on #7868 because some specs are really abusing the API for autocorrection.